Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/#315: 키워드 구독 기능 추가 #319

Merged
merged 20 commits into from
Feb 12, 2024
Merged

Feat/#315: 키워드 구독 기능 추가 #319

merged 20 commits into from
Feb 12, 2024

Conversation

pp449
Copy link
Member

@pp449 pp449 commented Feb 2, 2024

🤠 개요

  • closes: Feat: 키워드 구독 기능 추가 #315
  • 마이페이지에 알림 설정, 키워드 알림 설정, 자주 묻는 질문 추가
  • 알림 설정의 경우 다음 PR 에서 작업예정 (현재 기능 X)
  • 자주 묻는 질문 클릭 시 FAQ 페이지로 이동

💫 설명

키워드 알림

  • 우선 마이페이지의 알림 토글을 ON 해야 부가적인 알림 설정, 키워드 알림 설정 이 보이도록 구현

키워드 알림 설정

  • 각 키워드의 최대 글자수는 최대 15글자로 설정
  • 키워드 설정은 최대 5개까지 설정
  • 키워드의 글자수는 최소 2글자
  • 중복된 키워드 설정 불가
  • 엔터로 입력 가능
  • X 버튼을 이용해 취소 가능

📷 스크린샷 (Optional)

Screen.Recording.2024-02-03.at.12.21.21.AM.mov

pp449 added 14 commits February 2, 2024 03:10
종, 키보드, 느낌표 아이콘 추가
알림 설정, 키워드 알림 설정, 자주 묻는 질문으로 이동할 수 있는 카드 추가
자주 묻는 질문 클릭 시 faq 페이지로 라우팅하는 기능 추가
구독한 키워드 조회, 신규 키워드 구독, 키워드 삭제 요청 모킹
구독이 안된 경우, 2글자 이상이 안되는 경우, 중복된 키워드를 설정하려는 경우, 최대 키워드 개수를 넘은 경우, 서버 에러가 발생한 경우에 대한 메시지 추가
유저가 구독한 키워드를 조회하는 기능
새로 키워드를 구독하는 기능
구독한 키워드를 취소하는 기능 추가
전체 구독 알림설정을 해야 키워드 설정이 보이도록 변경
각 키워드의 최대 글자수는 최대 15글자로 설정
키워드 설정은 최대 5개까지 설정
키워드의 글자수는 최소 2글자
중복된 키워드 설정 불가
X 버튼 클릭 시 해당 키워드를 삭제할 수 있는 기능 추가
@pp449 pp449 added the ✨ feat label Feb 2, 2024
@pp449 pp449 requested a review from hwinkr February 2, 2024 15:53
@pp449 pp449 self-assigned this Feb 2, 2024
Copy link
Collaborator

@hwinkr hwinkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 93 to 99
<KeywordSubmit
disabled={!checkIsAvailable()}
isAvailable={checkIsAvailable()}
onClick={onClickSubmit}
>
등록
</KeywordSubmit>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

common 폴더에 있는 공통 컴포넌트 Button을 사용하지 않은 이유가 있나요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

사진의 '등록' 버튼에 대한 Button 인데 다른곳에 사용하는 버튼과 모양이 많이 다르다 생각해서 추가적인 CSS 요소가 많을거 같아 공통 컴포넌트를 사용하지 않았어요

공통 컴포넌트의 Button 쓰는게 더 좋을까요?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 고민이 많이 됐었는데, 일단은 common 폴더에 있는 버튼 컴포넌트를 최대한 재활용 하는 방향으로 사용하고 있긴 합니다. 테오의 프론트엔드 방에서도 질문을 했었는데 스타일이 다르다는 이유로 공통 컴포넌트를 사용하지 않고 매번 새로운 컴포넌트를 생성하기 보다 스타일을 외부에서 주입하면서 미리 구현해둔 공통 컴포넌트를 재사용하는 방향으로 간다고 하네요.. 저도 아직 어떤 방법에 더 옳은 방법인가에 대한 확신은 없지만 새로운 컴포넌트를 매번 만드는 것 보다 스타일을 주입하더라도 공통 컴포넌트를 재사용하는 방향이 더 괜찮은 것 같다는 생각은 하고 있어요~

image

Comment on lines 86 to 92
<KeywordInput
onChange={setInputKeyword}
placeholder={KEYWORD_PAGE.PLACEHOLDER}
maxLength={15}
onKeyDown={handleKeyDown}
value={inputKeyword}
/>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 onKeyDown 이벤트가 발생할 때마다 handleKeyDown 콜백 함수가 실행될 것 같은데, 이 방법보다는 form으로 감싸고 onSubmit 이벤트에 콜백 함수를 바인딩 하는건 어떨까요? 이렇게 하면 등록버튼, 엔터 모두 정상적으로 동작합니다

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

form 이 더 깔끔한거 같네요 수정할게요

fetchKeywords();
}, []);

const checkIsAvailable = () => (inputKeyword.length > 1 ? true : false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const checkIsAvailable = () => inputKeyword.length > 1

<MajorCard>
<Title>관리</Title>
<CardIconList isSubscribe={!!subscribe}>
<Icon kind="bell" color={THEME.PRIMARY} size="35" />{' '}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{' '}

이 빈 문자열은 뭔가요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prettier 에서 자동으로 가독성을 위해 추가한거 같은데 지울게요~

<CardList>
<span>알림 설정</span>
<ToggleButton
isOn={Boolean(subscribe)}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

토글 버튼을 사용할 때는 Boolean 객체를 사용하고, 아래에서는 !!을 사용한 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

엇 동일한 결과를 출력하긴 하는데 기분에따라 사용했나보네요,, 통일하겠습니다

</CardList>
</MajorCard>
<MajorCard>
<Title>관리</Title>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

관리 섹션 내부에 있는 CardIconList를 모두 isSubscribe를 통해서 조건부 렌더링을 하고 있는데, 자주 묻는 질문 탭 같은 경우는 전공이 설정 되어있지 않더라도 확인을 할 수 있는 정보이니 이렇게 하는 것 보다 일단 모두 렌더링을 한 후 구독 여부로

  • 토스트 메시지 출력 (전공 설정 되어있지 않음)
  • 해당 페이지로 이동 시키기 (전공 설정 되어있음)

이 2가지로 분기 처리하는 로직을 사용하는건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기에 대해서는 나중에 회의를 통해서 한 번 얘기해보는게 좋을거 같아요!

pp449 added 6 commits February 6, 2024 17:01
- 불필요한 코드 제거 {' '}
- boolean 반환코드 docker-compose up로 통일
- 키워드 구독 전송을 keydown -> form 제출 형식으로 변경
피드백에 따라 최대한 공통 컴포넌트를 사용할 수 있으면 사용하도록 기존에 있는 버튼 컴포넌트 사용
@pp449
Copy link
Member Author

pp449 commented Feb 10, 2024

피드백 수정 완료

Copy link
Collaborator

@hwinkr hwinkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM

@pp449 pp449 merged commit 2671e0b into dev Feb 12, 2024
1 check passed
@pp449 pp449 deleted the feat/#315 branch February 12, 2024 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat: 키워드 구독 기능 추가
2 participants